[test](nereids) stabilize flaky prune_bucket_with_bucket_shuffle_join#64530
Merged
yiguolei merged 1 commit intoJun 16, 2026
Merged
Conversation
With enable_nereids_distribute_planner=true the RIGHT OUTER JOIN distribution is non-deterministic between BUCKET_SHUFFLE and PARTITIONED. The choice is sticky within a JDBC connection, so the existing retry(120, 1000) (which reuses the same connection) cannot escape PARTITIONED once a connection lands there, and the BUCKET_SHUFFLE assertion fails on a large fraction of runs. Both plans are correct; BUCKET_SHUFFLE just saves one exchange. Only run the bucket-shuffle-specific checks when the planner actually chose BUCKET_SHUFFLE; otherwise return early.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
I found one blocking issue in the test-only change.
Critical checkpoint conclusions:
- Goal/test proof: The PR aims to stabilize a flaky bucket-shuffle regression case. It removes the plan-shape flake, but the fallback branch now skips the only query-result check, so the test no longer proves Nereids result correctness when the planner chooses PARTITIONED.
- Scope/focus: The change is small, but returning from the whole suite is broader than needed; only the bucket-shuffle-specific exchange/tablet-pruning assertions should be skipped.
- Concurrency/lifecycle/config/compatibility/persistence/data writes/FE-BE variables/security: Not applicable; this is a single regression-test script change.
- Parallel paths: The legacy PARTITIONED explain path remains checked. The new Nereids PARTITIONED fallback lacks the result validation that still applies to both legal distributions.
- Special condition: The non-deterministic BUCKET_SHUFFLE vs PARTITIONED branch is documented, but the branch currently skips non-bucket-specific validation.
- Test coverage/results: Existing .out content is unchanged and correct for the query, but it becomes inert on fallback runs because order_qt_fillup_bucket is not executed.
- Observability/performance: Not applicable.
User focus: No additional user-provided review focus was specified.
| } | ||
| } | ||
| if (!bucketShuffleExplain.contains("RIGHT OUTER JOIN(BUCKET_SHUFFLE)")) { | ||
| return |
Contributor
There was a problem hiding this comment.
When this guard takes the PARTITIONED branch, the suite returns before order_qt_fillup_bucket sqlStr, so the existing -- !fillup_bucket -- block is never compared on exactly the runs this PR is stabilizing. The result check is not bucket-shuffle-specific; it should still execute for both legal distributions, with only the exchange/tablet-pruning assertions skipped.
Suggested change
| return | |
| order_qt_fillup_bucket sqlStr | |
| return |
yiguolei
approved these changes
Jun 16, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by anyone and no changes requested. |
github-actions Bot
pushed a commit
that referenced
this pull request
Jun 16, 2026
…#64530) ## Proposed changes Stabilize the flaky regression test `prune_bucket_with_bucket_shuffle_join`. ### Problem With `enable_nereids_distribute_planner=true`, the `RIGHT OUTER JOIN` in this case has a non-deterministic distribution: it can be planned as either `BUCKET_SHUFFLE` or `PARTITIONED`. Both plans are correct — `BUCKET_SHUFFLE` just has one fewer exchange. The choice is **sticky within a JDBC connection**: every `explain` on the same connection returns the same distribution. The regression framework reuses one connection per suite (`SuiteContext.getConnection()` caches it in a `ThreadLocal`), so the existing `retry(120, 1000)` retries on the same sticky connection and can never flip `PARTITIONED` → `BUCKET_SHUFFLE`. Once a run lands on `PARTITIONED`, the `assertTrue(result.contains("RIGHT OUTER JOIN(BUCKET_SHUFFLE)"))` assertion fails for all 120 retries → flaky failure. ### Fix After enabling the distribute planner, explain once and check whether the plan actually chose `BUCKET_SHUFFLE`: - if yes → run the existing bucket-shuffle-specific checks (single exchange, tablet pruning, result check); - if no → return early. This is a test-only change; it does not touch FE/BE planner behavior. Both distributions already produce correct results. ## Further comments The underlying non-determinism (benign tie-break vs. whether the planner should deterministically prefer `BUCKET_SHUFFLE`) is a separate planner question and is left as-is here; this PR only removes the flakiness from the regression case.
github-actions Bot
pushed a commit
that referenced
this pull request
Jun 16, 2026
…#64530) ## Proposed changes Stabilize the flaky regression test `prune_bucket_with_bucket_shuffle_join`. ### Problem With `enable_nereids_distribute_planner=true`, the `RIGHT OUTER JOIN` in this case has a non-deterministic distribution: it can be planned as either `BUCKET_SHUFFLE` or `PARTITIONED`. Both plans are correct — `BUCKET_SHUFFLE` just has one fewer exchange. The choice is **sticky within a JDBC connection**: every `explain` on the same connection returns the same distribution. The regression framework reuses one connection per suite (`SuiteContext.getConnection()` caches it in a `ThreadLocal`), so the existing `retry(120, 1000)` retries on the same sticky connection and can never flip `PARTITIONED` → `BUCKET_SHUFFLE`. Once a run lands on `PARTITIONED`, the `assertTrue(result.contains("RIGHT OUTER JOIN(BUCKET_SHUFFLE)"))` assertion fails for all 120 retries → flaky failure. ### Fix After enabling the distribute planner, explain once and check whether the plan actually chose `BUCKET_SHUFFLE`: - if yes → run the existing bucket-shuffle-specific checks (single exchange, tablet pruning, result check); - if no → return early. This is a test-only change; it does not touch FE/BE planner behavior. Both distributions already produce correct results. ## Further comments The underlying non-determinism (benign tie-break vs. whether the planner should deterministically prefer `BUCKET_SHUFFLE`) is a separate planner question and is left as-is here; this PR only removes the flakiness from the regression case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Stabilize the flaky regression test
prune_bucket_with_bucket_shuffle_join.Problem
With
enable_nereids_distribute_planner=true, theRIGHT OUTER JOINin this case has anon-deterministic distribution: it can be planned as either
BUCKET_SHUFFLEorPARTITIONED.Both plans are correct —
BUCKET_SHUFFLEjust has one fewer exchange.The choice is sticky within a JDBC connection: every
explainon the same connectionreturns the same distribution. The regression framework reuses one connection per suite
(
SuiteContext.getConnection()caches it in aThreadLocal), so the existingretry(120, 1000)retries on the same sticky connection and can never flipPARTITIONED→BUCKET_SHUFFLE. Once a run lands onPARTITIONED, theassertTrue(result.contains("RIGHT OUTER JOIN(BUCKET_SHUFFLE)"))assertion fails for all120 retries → flaky failure.
Fix
After enabling the distribute planner, explain once and check whether the plan actually
chose
BUCKET_SHUFFLE:pruning, result check);
This is a test-only change; it does not touch FE/BE planner behavior. Both distributions
already produce correct results.
Further comments
The underlying non-determinism (benign tie-break vs. whether the planner should
deterministically prefer
BUCKET_SHUFFLE) is a separate planner question and is leftas-is here; this PR only removes the flakiness from the regression case.